Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test updated build with docker-compose #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshmoore
Copy link
Member

@joshmoore joshmoore commented May 8, 2019

Beyond just using make for building a set of paired
omero-web and omero-web-standalone images, this allows
building within a self-contained docker-compose.yml.

see: #28

Questions:

  • default server to point at
  • whether or not to include a server & db container

Testing:

  • choose your server list (updating .env or setting the environment variable
  • run docker-compose up -d
  • check that you are using the latest build from merge-ci

cc: @manics @will-moore

Beyond just using make for building a set of paired
omero-web and omero-web-standalone images, this allows
building within a self-contained docker-compose.yml.

see: ome#28
joshmoore added a commit to joshmoore/omero-server-docker that referenced this pull request May 17, 2019
joshmoore added a commit to joshmoore/omero-server-docker that referenced this pull request May 17, 2019
@rgozim
Copy link
Member

rgozim commented May 28, 2019

This works on my machine, however there could be a potential build/run order issue. There is a github issue discussing how docker-compose doesn't support this use case.

The depends_on option is potential solution, however again, there is a github issue advising against it.

@manics might have more to say on it.

@manics
Copy link
Member

manics commented May 28, 2019

If the main aim is to test prebuilt merge-ci and latest-ci builds we could make it even easier for people by ensuring the containers are already built by devspace, either by

Another advantage is that you could test this using our production OMERO docker-compose.yml files by overriding the images instead of two more docker-compose.yml files for omero-web and omero-server.

@joshmoore
Copy link
Member Author

If the main aim is to test prebuilt merge-ci and latest-ci builds

I think the main driver was to add the necessary configurability to do everything you proposed in the via the makefile in docker-compose.yml itself. In fact, I could see moving away from the makefile in favor of docker-compose as the standard build mechanism. It also makes for a clear documentation of how everything is to be used.

we could make it even easier for people

I can definitely see having both :merge and :latest(-ci?) versions pushed regularly.

instead of two more docker-compose.yml files

Yeah, I'm not overly happy with the degree of cut-n-paste that docker-compose requires in general, but I think there's still value in having these files in the repo, especially if we can get rid of the cut-n-paste in Makefile. When I'm making changes to omero-{web,server}-docker, I often find myself wanting an in-place docker-compose.yml. (I've had local ones multiple times.)

@joshmoore
Copy link
Member Author

Note: the only thing this doesn't easily provide is a way to add more environment variables without modifying the docker-compose.yml itself. If you have ideas on how to add that customization, I'd be interested.

@manics
Copy link
Member

manics commented Jun 7, 2019

I'm still not convinced of the benefit of having a docker-compose.yml file in this repo, but if we're going ahead with it can you look into the potential ordering issue: #29 (comment) ?

@joshmoore
Copy link
Member Author

@manics, are you referring to https://stackoverflow.com/a/51425060/56887 ? I tried this:

diff --git a/docker-compose.yml b/docker-compose.yml
index 209f9c1..13d2b9e 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -17,10 +17,16 @@ services:
     image: ${REPO}/omero-web-standalone:${PREFIX}
     build:
       context: standalone
+      network: omero
       args:
...
+
+networks:
+  default:
+    name: omero

but it would bump our minimum requirement:

$ docker-compose pull
ERROR: The Compose file './docker-compose.yml' is invalid because:
networks.default value Additional properties are not allowed ('name' was unexpected)
services.web.build contains unsupported option: 'network'

I've not run into any ordering issues, i.e. the state of this PR has always worked for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants